Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move feat from %sqlrender to %sqlcmd snippets #681

Merged
merged 1 commit into from
Jun 30, 2023
Merged

Move feat from %sqlrender to %sqlcmd snippets #681

merged 1 commit into from
Jun 30, 2023

Conversation

bbeat2782
Copy link

@bbeat2782 bbeat2782 commented Jun 27, 2023

Describe your changes

Moved %sqlrender feature to @sqlcmd snippets, added FutureWarning to %sqlrender, updated documentations, and modified test functions to use @sqlcmd snippets

Issue number

Closes #647

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--681.org.readthedocs.build/en/681/

@bbeat2782
Copy link
Author

Closed the original PR and created a new PR due to the change in cmd functions location. The original PR is here.

Unresolved questions from the previous PR's comments

  1. Do we remove %sqlrender guide under API Reference?
Screen Shot 2023-06-27 at 4 38 54 PM

@edublancas
Copy link

Do we remove %sqlrender guide under API Reference?

yes, let's remove it

Copy link

@yafimvo yafimvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. Added some comments. Please also make sure to remove the %sqlrender guide

src/sql/magic.py Show resolved Hide resolved
src/sql/cmd/snippets.py Show resolved Hide resolved
@bbeat2782 bbeat2782 requested a review from yafimvo June 28, 2023 19:27
Copy link

@edublancas edublancas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please address the observations from the existing reviewers

tonykploomber
tonykploomber previously approved these changes Jun 28, 2023
Copy link

@tonykploomber tonykploomber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@bbeat2782
Copy link
Author

please address the observations from the existing reviewers

@edublancas I made the change (deleting double space in the magic-snippet.md) but unable click the re-request review button because the request has already been made. Other observations from other reviewers are all reflected.

print(chinstrap_sub_snippet)
```

This returns the stored snippet `chinstrap_sub`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this add some sentence like, Now, let's look at you to delete stored snippets.
Please verify any doc changes using Grammarly for typos/ syntax issues

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please click the Resolve Conversation button whenever you resolve a review comment

%sqlcmd snippets -d gentoo
```

This deletes the stored snippet `gentoo`.

To demonstrate `force-delete` let's create a snippet dependent on `chinstrap` snippet.
To demonstrate `force-delete` let's first confirm there is a snippet dependent on `chinstrap` snippet.
Copy link

@neelasha23 neelasha23 Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't confirm whether chinstrap_sub is dependent on chinstrap. It just displays the available snippets.
Maybe just mention, we have created the chinstrap_sub which is dependent on chinstrap. To delete chinstrap we need to use force-delete (please rephrase as needed)

In the given example, we demonstrated JupySQL's usage as a tool for managing large SQL queries in Jupyter Notebooks. It effectively broke down a complex query into smaller, organized parts, simplifying the process of analyzing a record store's sales database. By using JupySQL, users can easily maintain and reuse their queries, enhancing the overall data analysis experience.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly changed here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2023-06-29 at 8 44 05 AM

There is no change in the wording. The change is coming from deleting a double new line, and I just checked this occurs automatically when I open a markdown file in jupyter lab by opening it in notebook format.
I will add the new line back to remove this change.

Moved features, added FutureWarning, update API Reference documentation, modified test functions

update changelog

update changelog for #647

Update CHANGELOG.md

documentation change of moving sqlrender feature

manually relocating changes

resolving docs build failed

lint

fixing readthedocs

remove invalid snippet from documenation

subquery and more detailed error message added

lint

change in formatting

update toc

ci

using pretty_print

ci

delete double space from magic-snippet

Update CHANGELOG.md

change in documention

ci
@edublancas edublancas removed the request for review from yafimvo June 30, 2023 14:25
@edublancas edublancas merged commit 9a8aeae into ploomber:master Jun 30, 2023
tl3119 added a commit to tl3119/jupysql that referenced this pull request Jun 30, 2023
Move feat from %sqlrender to %sqlcmd snippets (ploomber#681)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move feature from %sqlrender -> %sqlcmd snippets
5 participants